feat(feedback): implement shake gesture detection#5150
feat(feedback): implement shake gesture detection#5150
Conversation
Adds SentryShakeDetector (accelerometer-based) and ShakeDetectionIntegration that shows the feedback dialog when a shake is detected. Controlled by SentryFeedbackOptions.useShakeGesture (default false). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
🤖 This preview updates automatically when you update the PR. |
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ee747ae | 357.79 ms | 421.84 ms | 64.05 ms |
| 22f4345 | 307.87 ms | 354.51 ms | 46.64 ms |
| dc4cc7a | 361.10 ms | 439.53 ms | 78.43 ms |
| a416a65 | 316.52 ms | 359.67 ms | 43.15 ms |
| d15471f | 310.66 ms | 368.19 ms | 57.53 ms |
| 9fbb112 | 361.43 ms | 427.57 ms | 66.14 ms |
| dba088c | 328.51 ms | 423.79 ms | 95.28 ms |
| 27d7cf8 | 397.90 ms | 498.65 ms | 100.75 ms |
| 951caf7 | 323.66 ms | 392.82 ms | 69.16 ms |
| a416a65 | 295.53 ms | 373.74 ms | 78.21 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 22f4345 | 1.58 MiB | 2.29 MiB | 719.83 KiB |
| dc4cc7a | 1.58 MiB | 2.19 MiB | 619.28 KiB |
| a416a65 | 1.58 MiB | 2.12 MiB | 555.26 KiB |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| 9fbb112 | 1.58 MiB | 2.11 MiB | 539.18 KiB |
| dba088c | 1.58 MiB | 2.13 MiB | 558.99 KiB |
| 27d7cf8 | 1.58 MiB | 2.12 MiB | 549.42 KiB |
| 951caf7 | 1.58 MiB | 2.13 MiB | 558.77 KiB |
| a416a65 | 1.58 MiB | 2.12 MiB | 555.26 KiB |
Previous results on branch: antonis/feedback-shake
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2052d5f | 331.52 ms | 401.71 ms | 70.19 ms |
| d743634 | 404.69 ms | 507.35 ms | 102.66 ms |
| 395b61e | 318.16 ms | 373.86 ms | 55.70 ms |
| ba57364 | 317.45 ms | 360.35 ms | 42.90 ms |
| 3a14925 | 327.15 ms | 380.52 ms | 53.37 ms |
| 90c01e9 | 314.38 ms | 370.51 ms | 56.13 ms |
| 58a9026 | 330.31 ms | 370.45 ms | 40.13 ms |
| a1a1e57 | 323.92 ms | 367.22 ms | 43.31 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2052d5f | 1.58 MiB | 2.29 MiB | 726.85 KiB |
| d743634 | 0 B | 0 B | 0 B |
| 395b61e | 1.58 MiB | 2.29 MiB | 727.32 KiB |
| ba57364 | 1.58 MiB | 2.29 MiB | 727.15 KiB |
| 3a14925 | 1.58 MiB | 2.29 MiB | 727.39 KiB |
| 90c01e9 | 1.58 MiB | 2.29 MiB | 727.37 KiB |
| 58a9026 | 1.58 MiB | 2.29 MiB | 727.33 KiB |
| a1a1e57 | 1.58 MiB | 2.29 MiB | 727.28 KiB |
- Add volatile/AtomicLong for thread-safe cross-thread field access - Use SystemClock.elapsedRealtime() instead of System.currentTimeMillis() - Use SENSOR_DELAY_NORMAL for better battery efficiency - Add multi-shake counting (2+ threshold crossings within 1.5s window) - Handle deferred init for already-resumed activities - Wrap showDialog() in try-catch to prevent app crashes - Improve activity transition handling in onActivityPaused - Mark SentryShakeDetector as @ApiStatus.Internal - Add unit tests for SentryShakeDetector and ShakeDetectionIntegration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
antonis
requested review from
adinauer,
lcian,
markushi and
romtsn
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
… shakes Track dialog visibility with an isDialogShowing flag that is set before showing and cleared via the onFormClose callback when the dialog is dismissed. Double-checked on both sensor and UI threads to avoid races. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
sentry-android-core/src/main/java/io/sentry/android/core/ShakeDetectionIntegration.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
… growth Save the user's original onFormClose once during register() and restore it after each dialog dismiss, instead of wrapping it with a new lambda each time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Outdated
Show resolved
Hide resolved
…ck flag If showDialog silently fails (e.g. activity destroyed between post and execution), isDialogShowing would stay true forever, permanently disabling shake-to-feedback. Reset it in onActivityPaused since the dialog cannot outlive its host activity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
…ActivityDestroyed AlertDialog survives pause/resume cycles (e.g. screen off/on), so resetting isDialogShowing in onActivityPaused allowed duplicate dialogs. Move the reset to onActivityDestroyed where the dialog truly cannot survive. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
…back on error - Only reset isDialogShowing in onActivityDestroyed when it's the activity that hosts the dialog, not any unrelated activity. - Restore originalOnFormClose in the catch block when showDialog throws. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| /** | ||
| * Detects shake gestures and shows the user feedback dialog when a shake is detected. Only active |
There was a problem hiding this comment.
I know the end goal is to add this to improve user feedback, but the way it is, it's doing nothing related to user feedback.
Might be a good idea to rename this to FeedbackShakeIntegration so the name of it self explains the usage of it, instead of leaving it generic, what do you think?
| (Application) context, buildInfoProvider, activityFramesTracker)); | ||
| options.addIntegration(new ActivityBreadcrumbsIntegration((Application) context)); | ||
| options.addIntegration(new UserInteractionIntegration((Application) context, loadClass)); | ||
| options.addIntegration(new ShakeDetectionIntegration((Application) context)); |
There was a problem hiding this comment.
Q: is it possible to only add when useShakeGesture is set to true?
There was a problem hiding this comment.
At the time installDefaultIntegrations runs in AndroidOptionsInitializer, the user's configuration callback hasn't executed yet and useShakeGesture is still false. That's why register() checks the option later. This is the same pattern used by other integrations (e.g., UserInteractionIntegration is always added but checks its options in register())
sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java
Outdated
Show resolved
Hide resolved
| sensorManager.unregisterListener(this); | ||
| sensorManager = null; | ||
| } | ||
| listener = null; |
There was a problem hiding this comment.
q: why not set listener null first?
Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Outdated
Show resolved
Hide resolved
Sentry Build Distribution
|
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
Allow enabling shake gesture via AndroidManifest.xml: <meta-data android:name="io.sentry.feedback.use-shake-gesture" android:value="true" /> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
…og is showing onActivityPaused always fires before onActivityDestroyed. Without this fix, currentActivity was set to null in onPause, making the cleanup condition in onActivityDestroyed (activity == currentActivity) always false. This left isDialogShowing permanently stuck as true, disabling shake-to-feedback for the rest of the session. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The detector was constructed with NoOpLogger and the logger field was final, so all diagnostic messages (sensor unavailable warnings) were silently swallowed. Now init(context, logger) updates the logger from SentryOptions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sentry Build Distribution
|
…java into antonis/feedback-shake
Thank you for the feedback @romtsn 🙇
Good idea 👍 Added with 65122ca |
Sentry Build Distribution
|
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java
Show resolved
Hide resolved
- Clear currentActivity in onActivityDestroyed to prevent holding a stale reference to a destroyed activity context - Reset shakeCount and firstShakeTimestamp in stop() to prevent cross-session false triggers across pause/resume cycles Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java
Show resolved
Hide resolved
Sentry Build Distribution
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
When a dialog is showing on Activity A and the user navigates to Activity B (e.g. via notification), onActivityResumed(B) overwrites currentActivity. Later onActivityDestroyed(A) can't match and cleanup never runs, leaving isDialogShowing permanently stuck. Now we detect this in onActivityResumed and clean up proactively. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Outdated
Show resolved
Hide resolved
Sentry Build Distribution
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java
Show resolved
Hide resolved
The onFormClose lambda was reading previousOnFormClose field at dismiss time. If onActivityResumed or onActivityDestroyed already restored and nulled the field, the lambda would overwrite onFormClose with null. Now captured as a local variable at dialog creation time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sentry Build Distribution
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java
Show resolved
Hide resolved
When close() is called while a dialog is showing, lifecycle callbacks are unregistered so onActivityDestroyed cleanup won't fire. Restore previousOnFormClose and reset dialog state in close() to prevent the callback from being permanently overwritten. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| handlerThread = new HandlerThread("sentry-shake"); | ||
| handlerThread.start(); | ||
| final Handler handler = new Handler(handlerThread.getLooper()); | ||
| sensorManager.registerListener(this, accelerometer, SensorManager.SENSOR_DELAY_NORMAL, handler); |
There was a problem hiding this comment.
Calling start() twice leaks previous HandlerThread
Medium Severity
SentryShakeDetector.start() overwrites the handlerThread field with a new HandlerThread without stopping/quitting the previous one. If start() is called twice without an intervening stop(), the old HandlerThread leaks and continues running. While FeedbackShakeIntegration always calls stop() before start(), this class is public and explicitly designed for reuse by React Native and other hybrid SDKs (per the PR description). External consumers calling start() on an already-started detector will silently leak threads.
There was a problem hiding this comment.
Duplicate of previous finding (see this reply). The class is @ApiStatus.Internal and all call sites in FeedbackShakeIntegration call stopShakeDetection() before startShakeDetection() (which itself calls stop() first at line 131). No thread leak is possible.
Sentry Build Distribution
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| } | ||
| previousOnFormClose = null; | ||
| }); | ||
| new SentryUserFeedbackDialog.Builder(active).create().show(); |
There was a problem hiding this comment.
Missing activity validity check before showing dialog
Low Severity
The shake callback captures currentActivity on the background handler thread and posts to runOnUiThread without first checking isFinishing() or isDestroyed() on the activity. Because onActivityPaused/onActivityDestroyed run on the main thread, there's a race window where the activity becomes invalid between capturing active on the handler thread and the runnable executing on the UI thread. This results in a WindowManager.BadTokenException when show() is called. While the try/catch(Throwable) prevents a crash, the codebase's established pattern in ScreenshotUtils.isActivityValid() proactively checks !activity.isFinishing() && !activity.isDestroyed() to avoid this. An isFinishing()/isDestroyed() guard inside the runOnUiThread lambda (before SentryUserFeedbackDialog.Builder) would prevent unnecessary error-level logging.
| } | ||
| currentActivity = activity; | ||
| startShakeDetection(activity); | ||
| } |
There was a problem hiding this comment.
Concurrent dialog wrappers corrupt onFormClose callback state
Low Severity
When onActivityResumed fires for a new activity while a dialog is showing on the old one, it resets isDialogShowing to false and restores onFormClose to previousOnFormClose. If the user then shakes on the new activity, a second dialog is created with a fresh onFormClose wrapper. When the old dialog (on the old activity) is eventually dismissed, its captured wrapper overwrites the global onFormClose with the original callback, destroying the new dialog's wrapper. This means the new dialog's dismiss won't reset isDialogShowing, potentially leaving the integration in a stuck state where no further shake dialogs can be shown.
Additional Locations (1)
4 participants
📜 Description
Implements shake gesture detection for the user feedback form. When
useShakeGestureis enabled inSentryFeedbackOptions, shaking the device opens the feedback form.The implementation uses the device's accelerometer (via
SensorManager) with a 2.7G threshold and 1-second cooldown. A newShakeDetectionIntegrationregisters lifecycle callbacks to start/stop the detector when the activity resumes/pauses.Note: this is exposed to be used on React Native too getsentry/sentry-react-native#5754
💡 Motivation and Context
The
useShakeGestureproperty already existed inSentryFeedbackOptionsbut was never wired up. This PR implements the feature. The implementation is placed here (sentry-java) rather than in each SDK separately so it can be reused by all SDKs that embed the feedback UI (React Native, Flutter, .NET MAUI, Unity).No special permissions are required —
TYPE_ACCELEROMETERis not a protected sensor. OnlyBODY_SENSORSrequires a permission.💚 How did you test it?
SentryShakeDetectorandShakeDetectionIntegration📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
sentry-react-native will remove its own
RNSentryShakeDetectorand delegate to this class instead.